Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request Call placeholder & messages updated #6178

Merged

Conversation

Santhosh-Sellavel
Copy link
Collaborator

Details

Just place holder & phone number error message copy updates for Request a Call Modal.

Fixed Issues

$ #5945

Tests & QA Steps

  1. Go to Concierge Chat
  2. Click on 📞 button in the chat header to open Request a Call Modal
  3. New place holder for Phone Number field. 2109400803
  4. Click call without entering any number to check the new error messages.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screenshot 2021-11-03 at 2 43 07 AM

Screenshot 2021-11-03 at 2 31 19 AM

Mobile Web

Simulator Screen Shot - iPhone 12 - 2021-11-03 at 02 25 36

Simulator Screen Shot - iPhone 12 - 2021-11-03 at 02 30 20

iOS

Simulator Screen Shot - iPhone 12 - 2021-11-03 at 02 24 10

Android

Screenshot_1635886131

Screenshot_1635886789

Desktop

@Santhosh-Sellavel Santhosh-Sellavel requested a review from a team as a code owner November 2, 2021 21:16
@MelvinBot MelvinBot requested review from chiragsalian and removed request for a team November 2, 2021 21:16
@Santhosh-Sellavel
Copy link
Collaborator Author

Santhosh-Sellavel commented Nov 2, 2021

@chiragsalian for the new messages text alignment looks odd.

@chiragsalian
Copy link
Contributor

Good point. I think the error message should maybe be center-aligned. But it's better to rope in design for this so cc-ing @Expensify/design for thoughts/suggestions on how the error message should appear.

@shawnborton
Copy link
Contributor

I think the left-alignment is fine as it is.

@Santhosh-Sellavel
Copy link
Collaborator Author

@chiragsalian bump!

@Santhosh-Sellavel
Copy link
Collaborator Author

Seems the design team is okay with this!

src/languages/en.js Outdated Show resolved Hide resolved
src/languages/es.js Outdated Show resolved Hide resolved
@Santhosh-Sellavel
Copy link
Collaborator Author

Will update the PR & let you know

@Santhosh-Sellavel
Copy link
Collaborator Author

Updated PR

@Santhosh-Sellavel
Copy link
Collaborator Author

@chiragsalian PR updated

Copy link
Contributor

@chiragsalian chiragsalian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍. We're on a merge freeze at the moment till Monday so I'll merge it then 🙂

@chiragsalian chiragsalian merged commit 9541bb1 into Expensify:main Nov 16, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @chiragsalian in version: 1.1.15-18 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.16-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants